Skip to content

Conversation

@zm711
Copy link
Contributor

@zm711 zm711 commented Jan 23, 2024

Fixes #1371.

As explained in the issue the structures are indexed different so we need to +1 to correctly index. Add comment to explain this with links.

@zm711
Copy link
Contributor Author

zm711 commented Jan 23, 2024

@h-mayorquin, I know you read over and help with the dll for plexon2. Do you want to double check this fix?

@alejoe91 alejoe91 added this to the 0.13.0 milestone Jan 24, 2024
@alejoe91 alejoe91 changed the title Fix conversion between C++ tm structure and python datetime Plexon2: Fix conversion between C++ tm structure and python datetime Jan 26, 2024
@apdavison apdavison merged commit 415845e into NeuralEnsemble:master Jan 26, 2024
Copy link
Contributor

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that being off by a second with the leap second is fine. Glad that @alejoe91 had the time to check it.

@zm711
Copy link
Contributor Author

zm711 commented Jan 26, 2024

@h-mayorquin, my point for leap second is that based on my reading Python datetime will not accept the leap second value

tm can return 60 or 61 to count as leap seconds, but datatime second only accepts 0..59. So in the case that someone records on a leap second then this IO could fail to annotate the date.

@zm711 zm711 deleted the plexon2 branch January 26, 2024 16:06
@h-mayorquin
Copy link
Contributor

What I mean is to just substract 1 and make it 60 in that case. The other solution would be to propagate all the way up to year in th worse case scenario which is indded difficult as I think you were pointing out.

But also, having the current code just failn and then have someone coming here to tell us so we can device a better solutio nwith an example in our hands seems like a good option : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plexon .pl2 files fail to load if it is recorded in January; files from other months have month created off-by-one

4 participants